Skip to content

[RUMF-848] Expose internal types and constants to be used in Datadog app#727

Merged
BenoitZugmeyer merged 12 commits intomasterfrom
benoit/rum-recorder-internal
Feb 11, 2021
Merged

[RUMF-848] Expose internal types and constants to be used in Datadog app#727
BenoitZugmeyer merged 12 commits intomasterfrom
benoit/rum-recorder-internal

Conversation

@BenoitZugmeyer
Copy link
Member

Motivation

Expose internal types to be used in Datadog app

Changes

Testing


I have gone over the contributing documentation.

@BenoitZugmeyer BenoitZugmeyer requested a review from a team as a code owner February 9, 2021 18:03
@BenoitZugmeyer BenoitZugmeyer changed the title hop Expose internal types to be used in Datadog app Feb 9, 2021
@BenoitZugmeyer BenoitZugmeyer changed the title Expose internal types to be used in Datadog app [RUMF-848] Expose internal types to be used in Datadog app Feb 9, 2021
@BenoitZugmeyer BenoitZugmeyer changed the title [RUMF-848] Expose internal types to be used in Datadog app [RUMF-848] Expose internal types and constants to be used in Datadog app Feb 9, 2021
@BenoitZugmeyer BenoitZugmeyer force-pushed the benoit/rum-recorder-internal branch from 1cecbc3 to d6b10f9 Compare February 9, 2021 18:07
@BenoitZugmeyer BenoitZugmeyer force-pushed the benoit/rum-recorder-internal branch from d6b10f9 to a162dd3 Compare February 10, 2021 09:11
@codecov-io
Copy link

Codecov Report

Merging #727 (f55db89) into master (c119e80) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #727   +/-   ##
=======================================
  Coverage   80.77%   80.77%           
=======================================
  Files          72       72           
  Lines        3745     3745           
  Branches      877      877           
=======================================
  Hits         3025     3025           
  Misses        720      720           
Impacted Files Coverage Δ
packages/rum-recorder/src/domain/privacy.ts 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c119e80...f55db89. Read the comment docs.

@BenoitZugmeyer BenoitZugmeyer force-pushed the benoit/rum-recorder-internal branch from f55db89 to 051b5e9 Compare February 10, 2021 11:08
@BenoitZugmeyer BenoitZugmeyer force-pushed the benoit/rum-recorder-internal branch from 051b5e9 to 86a13a1 Compare February 10, 2021 11:09
@BenoitZugmeyer BenoitZugmeyer force-pushed the benoit/rum-recorder-internal branch from f45be84 to 62dd2d1 Compare February 10, 2021 15:04
@BenoitZugmeyer BenoitZugmeyer force-pushed the benoit/rum-recorder-internal branch from 62dd2d1 to a9ac8f1 Compare February 10, 2021 15:11
* Add some documentation
* Enforce it on all code that should be added to the packages
* Reword a few functions
* Make sure only files with side effects can import files with side
effects
* Allow `new WeakMap()` and `Object.keys()` calls
* Replace unnecessary RegExp constructor with a regexp value
* Disable the rule on 'mutationBuffer', as I want to remove this in the
future.
* Remove unused `freezePage` function
@BenoitZugmeyer BenoitZugmeyer force-pushed the benoit/rum-recorder-internal branch from 8c252f6 to 01f2da1 Compare February 11, 2021 09:39
Comment on lines +42 to 44
// TODO: remove this global MutationBuffer instance
// eslint-disable-next-line local-rules/enforce-declarative-modules
export const mutationBuffer = new MutationBuffer()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to remove this in a separate PR, because it involves a bit of rearchitecturing of RRWeb codebase

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See PR #728

* rename the rule to `disallow-side-effects`
* adjust doc comments
* remove unneeded eslint-disable comment
* improve rule description and report message
@BenoitZugmeyer BenoitZugmeyer force-pushed the benoit/rum-recorder-internal branch from 0660bdc to 44d2b84 Compare February 11, 2021 14:43
Copy link
Collaborator

@bcaudan bcaudan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work 🎉

@BenoitZugmeyer BenoitZugmeyer merged commit b07afeb into master Feb 11, 2021
@BenoitZugmeyer BenoitZugmeyer deleted the benoit/rum-recorder-internal branch February 11, 2021 15:00
webNeat pushed a commit that referenced this pull request Feb 15, 2021
…app (#727)

* [RUMF-848] expose internal types and constants to be used in Datadog app

* [RUMF-848] add a ESLint rule to enforce declarative modules

* [RUMF-848] allow using 'import type' from any file in restricted files

* 🚚 [RUMF-848] move types and constants to respect the new ESLint rule

* [RUMF-848] adjust ESLint rule

* Add some documentation
* Enforce it on all code that should be added to the packages
* Reword a few functions
* Make sure only files with side effects can import files with side
effects
* Allow `new WeakMap()` and `Object.keys()` calls

* 🚚 [RUMF-848] remove tracekit IIFE by spliting it into different modules

* [RUMF-848] replace tracekit report.* with proper functions

* [RUMF-848] replace tracekit computeStackTrace.* with proper functions

* [RUMF-848] move LogsUserConfiguration in logs.ts

* [RUMF-848] fix last RRWeb issues

* Replace unnecessary RegExp constructor with a regexp value
* Disable the rule on 'mutationBuffer', as I want to remove this in the
future.
* Remove unused `freezePage` function

* 👌 [RUMF-848] review adjustments

* rename the rule to `disallow-side-effects`
* adjust doc comments
* remove unneeded eslint-disable comment
* improve rule description and report message
BenoitZugmeyer added a commit that referenced this pull request Apr 22, 2021
This logic was introduced to allow RRWeb users to "pause" the recording
by using the public method `freezePage`. This method has been removed in
PR #727, and it appears that the `freeze` logic is now unneeded: since
the "fullsnapshot" logic is synchronous, no DOM mutation can happen or
be processed while the document snapshot is taken.
BenoitZugmeyer added a commit that referenced this pull request Apr 23, 2021
This logic was introduced to allow RRWeb users to "pause" the recording
by using the public method `freezePage`. This method has been removed in
PR #727, and it appears that the `freeze` logic is now unneeded: since
the "fullsnapshot" logic is synchronous, no DOM mutation can happen or
be processed while the document snapshot is taken.
@codecov-commenter
Copy link

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.77%. Comparing base (ce55e19) to head (f55db89).

⚠️ Current head f55db89 differs from pull request most recent head 44d2b84

Please upload reports for the commit 44d2b84 to get more accurate results.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #727      +/-   ##
==========================================
- Coverage   82.64%   80.77%   -1.87%     
==========================================
  Files          72       72              
  Lines        3745     3745              
  Branches      877      877              
==========================================
- Hits         3095     3025      -70     
- Misses        650      720      +70     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants